-
Notifications
You must be signed in to change notification settings - Fork 263
Add noise generators and improve their distribution #755
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
should it? I've never seen seek as reproducing something rather navigating in an underlying stream. I would say the seek implementation for noise like this should do nothing. But if we do go on with struct DisableSeek;
impl Source for DisableSeek {
....
fn try_seek() -> Result<() , _> {
Ok(())
}
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did not have time to look at the implementation in depth I do have some feedback/questions regarding the API and tests.
Here is an example of what would happen if we would "eat" seeks: // Create with a fixed seed for determinism
let mut noise = pink::new_with_seed(44100, 12345);
// Play for a while, changing internal state
for _ in 0..1000 { noise.next(); }
// Rewind to the beginning
noise.try_seek(Duration::from_secs(0)).unwrap();
// Create another generator with the same seed
let mut noise2 = pink::new_with_seed(44100, 12345);
// Noises should be the same, but are not
assert_eq!(noise.next(), noise2.next()); ...where you've got me however is that this is also true for my proposed implementation of Thinking out loud: maybe we should remove |
I think a deterministic noise gen is pretty neat but I struggle to think of a use case for it. Noise pretty much sounds the same regardless of seed right? If we can not find a good use case now I would propose we scrap determinism. We can always add |
Yes, let's scrap it. |
The commit improves and expands the noise generation capabilities by adding new types and fixing distribution issues in existing ones. Key changes: - Add more generators: Gaussian, triangular, blue, brownian, violet, velvet - Fix white noise uniform distribution - Fix pink noise sampling rate issues - Make noise generators properly deterministic with seeds - Add comprehensive tests for noise generator properties - Improve documentation with detailed usage guidance This provides a complete suite of high-quality noise generators for audio synthesis, testing, and dithering applications.
- Rename noise generators to PascalCase types (e.g. WhiteUniform, Pink) - Move all noise generators under `source::noise` module - Deprecate `white()` and `pink()` in favor of `noise::WhiteUniform::new()` and `noise::Pink::new()` - Remove legacy noise generator functions/types from prelude - Update `rstest`, `rstest_reuse` to fix templates in unit tests - Refactor and expand noise generator documentation and examples - Move and consolidate noise generator tests into `src/source/noise.rs` - Update `examples/noise_generator.rs` to use new API and add descriptions
94f0f53
to
377fbb9
Compare
Thanks for all the hard work, it looks really nice. Only thing I'm still unsure about is the seeking behavior. You've explained you want seeking to be reproducible/deterministic. I think we currently use seeking mostly for decoders. Effects just forward the seek or adjust it in the case of |
[emphasis mine] Are you referring to But yes, you're probably right to not be too academic about this. It's probably OK to "eat" the seek and always return |
your right! In my head that was already fixed. Well we should be able to fix that now that we have |
- Merge basic and seekable noise source macros into `impl_noise_source` - Simplify documentation and trait implementations for all noise generators - Update tests to reflect unified seeking support
OK, a952f97 should do it, making all noise generators seekable. Once you agree, I can squash merge. |
I think this epic is now done :) Lets get it merged! I'll see if I can finish up get 0.21 ready for release tomorrow. |
Fixes
The current
WhiteNoise
generator has a problem where its distribution is not at all in range of[-1.0, 1.0]
due to precision loss, truncation and bias to zero from doing this:This PR fixes that by using a proper
f32
generated uniform distribution.Then the current
PinkNoise
builds on top of the incorrectWhiteNoise
but worse: has coefficients that are valid for 44.1 kHz only. This PR fixes that by algorithmically generating proper pink noise.Finally,
try_seek
should provide deterministic seeking (same results after seeking) andPinkNoise
cannot provide that. This PR correctly implementstry_seek
for noise generators that are deterministic (or uniformly random).New noise generators
Threw in some freebies:
Including documentation on when you'd want to use which.
I noticed that docs.rs didn't document the feature-gated noise generators, so I've set it to generate documentation for all features.
Advanced construction options
The current API remains as-is:
source::white()
will give you aWhiteGenerator<SmallRng>
. But you see that I made the generators generic for more choice to the user:source::WhiteNoise::<R: Rng>::new{_with_seed}()
.This now provides a rather complete suite of high-quality noise generators for audio synthesis, testing, and dithering.